Skip to content

fix(tests): mark the terraform-cloud target Terraform-only in the matrix - #430

Merged
jsteinich merged 2 commits into
open-constructs:mainfrom
jsteinich:fix/terraform-cloud-runtime-pin
Sep 14, 2026
Merged

jsteinich merged 2 commits into
open-constructs:mainfrom
jsteinich:fix/terraform-cloud-runtime-pin

Conversation

@jsteinich

Copy link
Copy Markdown
Contributor

Related issue

Fixes #392. Unblocked by #405, which landed the per-test runtime pinning this builds on.

Description

test/typescript/terraform-cloud/test.ts feeds TERRAFORM_VERSION — set from the matrix axis — into the terraformVersion attribute of an HCP Terraform workspace. If the matrix ever resolves that target to OpenTofu, the test asks HCP Terraform for a workspace pinned to a Terraform version that does not exist, and there is no correct value to pass: that API executes Terraform remotely and has no OpenTofu equivalent.

The target is declared Terraform-only in the matrix builder, and the builder throws if it ever resolves otherwise:

const terraformOnly = new Set([
  // HCP Terraform's workspace API pins a Terraform version and has no OpenTofu equivalent.
  "typescript/terraform-cloud/test.ts",
]);

The check runs against each target's resolved runtime rather than against pinnedRuntimes, so it catches both a pin and a hypothetical future non-Terraform default axis. Filtering the default axis instead would have silently dropped the target — the same class of quiet misconfiguration the issue is about.

Why not the other two options the issue lists

Skip inside the test trades a loud failure for a green job that asserted nothing. If someone later pinned this target to tofu, CI would read as OpenTofu coverage while proving nothing.

Hardcode terraformVersion to a fixed release is the one worth pushing back on. Today TERRAFORM_VERSION makes the workspace's remote Terraform version match the local CLI the matrix selected. Hardcoding one side introduces deliberate local/remote skew on every entry that isn't the hardcoded version, and collapses remote-version coverage to one version — so both matrix entries would create identical workspaces and half the jobs become redundant. It also would not settle a tofu pin: you would still be driving a remote-execution Terraform workspace with tofu, failing confusingly rather than loudly. terraform-cloud/test.ts is untouched.

Second commit: making the guard trustworthy

Both this guard and pinnedRuntimes from #405 are keyed by test path, and a key matching no discovered target makes them silently inert. Verified: with a typo'd terraformOnly key, a tofu-pinned terraform-cloud entry sails through exactly as if the guard were absent. That defeats the purpose of adding it, so the keys are now validated once against the discovered target list, after discovery, and both maps fail closed.

Testing

tools/build-test-matrix.mjs does not run natively on Windows (execFileSync("npx") needs .cmd; the testDir prefix strip assumes forward slashes), so the real logic was run against a stubbed discovery list — only the execFileSync block is replaced, with stubs built using the same `${testDir}/` prefix so the prefix-strip path is still exercised. Pins, guards, HCL detection and binary naming are untouched source.

case result
default config identical to baseline; terraform-cloud gets terraform1.5.7 + terraform1.16.1
tofu pin on terraform-cloud Error: typescript/terraform-cloud/test.ts is Terraform-only, but the matrix resolved it to opentofu 1.12.6.
non-Terraform default axis, no pin throws the same error — the claim in the code comment, proven rather than asserted
Terraform pin (1.5.5) resolves normally; guard does not over-fire
typo'd terraformOnly key Error: "…/tests.ts" is configured in pinnedRuntimes or terraformOnly but is not a discovered test target.
unrelated targets identical before/after

prettier --check and node --check clean.

Latent coupling elsewhere: checked, this is the only one

A repo-wide sweep for TERRAFORM_VERSION found test/typescript/terraform-cloud/test.ts (lines 7 and 47) to be the only test consumer; every other hit is a producer (integration.yml, provider-integration.yml) or a Docker build arg. A sweep of every process.env read under test/ turned up only TF_EXECUTE_LOCAL, TF_VAR_myvar, SWITCH_STACK, USE_CONSTRAINED_FUNCTION and TERRAFORM_CLOUD_* — none version- or product-coupled. It is also the only target touching app.terraform.io.

Not verified

The test itself cannot be run here: it needs TERRAFORM_CLOUD_TOKEN and talks to HCP Terraform. The stubbed harness proves the selection logic, not jest's discovery — so that the real target list is unchanged, and that the path key matches what jest --listTests emits, are confirmed by the first CI run of prepare-integration-tests. The key format is the same one the existing provider-features pin uses, and that pin resolves correctly in these runs.

Adjacent, deliberately not changed

examples.yml:171, pr-unit.yml:173, unit.yml:80 and provider-integration.yml:102 still string-concatenate "terraform${{ … }}" for TERRAFORM_BINARY_NAME, where integration.yml now uses matrix.binary. Those workflows have no OpenTofu axis today, so it is latent rather than broken.

🤖 Generated with Claude Code

jsteinich and others added 2 commits September 12, 2026 08:41
`test/typescript/terraform-cloud/test.ts` passes `TERRAFORM_VERSION` — the
matrix axis — as the `terraformVersion` of the HCP Terraform workspace it
creates. HCP Terraform runs Terraform remotely and exposes no OpenTofu
equivalent through that API, so a non-Terraform runtime has no correct value to
put there, and the resulting failure would say nothing about the cloud backend
the test covers. Closes open-constructs#392.

Since open-constructs#405 the matrix builder owns runtime selection per target, so the
constraint belongs there. `terraformOnly` is checked against each target's
*resolved* runtimes rather than against `pinnedRuntimes` alone, so both a pin
and a hypothetical non-Terraform default axis fail at matrix-build time with a
named target and product, before any job is scheduled.

Rejected the two alternatives:

- Skipping inside the test when the runtime is not Terraform trades a loud
  failure for a green job that asserted nothing. A pin added later would look
  like tofu coverage while proving nothing.
- Pinning `terraformVersion` to a fixed release decouples the remote version
  from the local CLI the matrix selects. Today they match; hardcoding one side
  introduces deliberate version skew across the remote backend on every entry
  and collapses remote-version coverage to a single version, so both matrix
  entries would create identical workspaces.

Terraform coverage is unchanged: the target still follows `tested` (1.5.7 and
1.16.1), and no emitted entry differs for any target.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The Terraform-only guard added in the previous commit, and the pinnedRuntimes
validation from open-constructs#405, are both keyed by test path. A key that matches no
discovered target makes them silently inert - a typo in `terraformOnly` lets
a tofu-pinned terraform-cloud entry through exactly as if the guard were not
there, which defeats the point of adding it.

Validated once against the discovered target list, after discovery, so both
maps fail closed.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@jsteinich
jsteinich marked this pull request as ready for review September 12, 2026 16:35
@jsteinich
jsteinich requested a review from a team as a code owner September 12, 2026 16:36

@so0k so0k left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No blocking or non-blocking findings.

  • tools/build-test-matrix.mjs validates both pinnedRuntimes and terraformOnly keys against Jest-discovered targets, preventing renamed or mistyped paths from silently disabling the configuration.
  • The Terraform-only check runs after runtime resolution, so it catches both an explicit OpenTofu pin and any future non-Terraform default runtime.
  • The designation is consistent with test/typescript/terraform-cloud/test.ts, which sends TERRAFORM_VERSION to HCP Terraform’s remote workspace API.
  • No workflow files or security-sensitive execution paths changed.

@jsteinich
jsteinich merged commit b0deefa into open-constructs:main Sep 14, 2026
261 checks passed
@jsteinich
jsteinich deleted the fix/terraform-cloud-runtime-pin branch September 14, 2026 13:07
@github-actions

Copy link
Copy Markdown
Contributor

I'm going to lock this pull request because it has been closed for 30 days. This helps our maintainers find and focus on the active issues. If you've found a problem that seems related to this change, please open a new issue and complete the issue template so we can capture all the details necessary to investigate further.

@github-actions github-actions Bot locked as resolved and limited conversation to collaborators Oct 15, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

test(tofu): terraform-cloud test feeds the matrix version into an HCP Terraform workspace, blocking an OpenTofu matrix leg

2 participants